feat(podcasts): short default brief length with seconds and unit picker#1501
Conversation
|
@CREDO23 is attempting to deploy a commit to the Rohan Verma's projects Team on Vercel. A member of the Team first needs to authorize it. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughAll podcast duration fields are migrated from minutes to seconds across the full stack. A new shared ChangesMinutes-to-Seconds Duration Migration
Quota Error Payload Update
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@surfsense_backend/app/podcasts/api/schemas.py`:
- Around line 37-46: The CreatePodcastRequest schema validates min_seconds and
max_seconds independently, allowing inverted ranges like min_seconds=100,
max_seconds=50 to pass validation at the API boundary. Add a Pydantic validator
(root_validator or field_validator depending on your Pydantic version) to the
CreatePodcastRequest class that enforces the constraint min_seconds <=
max_seconds, ensuring invalid duration ranges are rejected with a 422 validation
error at the API boundary instead of causing a 500 error later when
DurationTarget is instantiated.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 1916dad0-43a0-40f1-b4e1-5284e4534756
📒 Files selected for processing (14)
surfsense_backend/app/podcasts/api/routes.pysurfsense_backend/app/podcasts/api/schemas.pysurfsense_backend/app/podcasts/duration_limits.pysurfsense_backend/app/podcasts/generation/brief/config.pysurfsense_backend/app/podcasts/generation/brief/nodes.pysurfsense_backend/app/podcasts/generation/brief/propose.pysurfsense_backend/app/podcasts/generation/transcript/nodes.pysurfsense_backend/app/podcasts/schemas/spec.pysurfsense_backend/tests/integration/podcasts/conftest.pysurfsense_backend/tests/unit/podcasts/conftest.pysurfsense_backend/tests/unit/podcasts/test_renderer.pysurfsense_backend/tests/unit/podcasts/test_spec.pysurfsense_web/components/tool-ui/podcast/brief-review.tsxsurfsense_web/contracts/types/podcast.types.ts
| min_seconds: int = Field( | ||
| default=DEFAULT_MIN_SECONDS, | ||
| ge=MIN_DURATION_SECONDS, | ||
| le=MAX_DURATION_SECONDS, | ||
| ) | ||
| max_seconds: int = Field( | ||
| default=DEFAULT_MAX_SECONDS, | ||
| ge=MIN_DURATION_SECONDS, | ||
| le=MAX_DURATION_SECONDS, | ||
| ) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Check CreatePodcastRequest duration validation:"
rg -n -C2 'class CreatePodcastRequest|min_seconds|max_seconds|model_validator|_check_duration_order' surfsense_backend/app/podcasts/api/schemas.py
echo
echo "Check downstream DurationTarget construction that currently relies on ordered bounds:"
rg -n -C2 'DurationTarget\(|min_seconds=brief\.min_seconds|max_seconds=brief\.max_seconds' surfsense_backend/app/podcasts/generation/brief/nodes.pyRepository: MODSetter/SurfSense
Length of output: 922
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== Check complete CreatePodcastRequest class ==="
rg -A 30 'class CreatePodcastRequest' surfsense_backend/app/podcasts/api/schemas.py
echo
echo "=== Find and examine DurationTarget ==="
fd -e py | xargs rg -l 'class DurationTarget' 2>/dev/null | head -5
echo
echo "=== Check DurationTarget implementation ==="
rg -B 2 -A 15 'class DurationTarget' --type pyRepository: MODSetter/SurfSense
Length of output: 2683
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== Read full DurationTarget class ==="
rg -A 50 'class DurationTarget' surfsense_backend/app/podcasts/schemas/spec.pyRepository: MODSetter/SurfSense
Length of output: 2097
Enforce duration ordering in CreatePodcastRequest.
min_seconds and max_seconds are validated independently, so inverted ranges pass request validation and fail later when DurationTarget is instantiated in surfsense_backend/app/podcasts/generation/brief/nodes.py (line 82) with a 500 error. Reject this at the API boundary with a 422 validation error instead.
Suggested fix
-from pydantic import BaseModel, ConfigDict, Field
+from pydantic import BaseModel, ConfigDict, Field, model_validator
@@
class CreatePodcastRequest(BaseModel):
@@
max_seconds: int = Field(
default=DEFAULT_MAX_SECONDS,
ge=MIN_DURATION_SECONDS,
le=MAX_DURATION_SECONDS,
)
focus: str | None = Field(default=None, max_length=2000)
+
+ `@model_validator`(mode="after")
+ def _check_duration_order(self) -> "CreatePodcastRequest":
+ if self.max_seconds < self.min_seconds:
+ raise ValueError("max_seconds must be >= min_seconds")
+ return self🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@surfsense_backend/app/podcasts/api/schemas.py` around lines 37 - 46, The
CreatePodcastRequest schema validates min_seconds and max_seconds independently,
allowing inverted ranges like min_seconds=100, max_seconds=50 to pass validation
at the API boundary. Add a Pydantic validator (root_validator or field_validator
depending on your Pydantic version) to the CreatePodcastRequest class that
enforces the constraint min_seconds <= max_seconds, ensuring invalid duration
ranges are rejected with a 422 validation error at the API boundary instead of
causing a 500 error later when DurationTarget is instantiated.
Billable calls now raise quota errors with balance_micros instead of used_micros/limit_micros; update mocks so CI passes on main.
Remove duplicate typing import and format legacy minute coercion guard.
Summary
min_seconds/max_seconds(defaults 20s–30s) with backward-compatible loading of legacymin_minutesspecs in JSONB.Test plan
uv run pytest tests/unit/podcasts/test_spec.pyuv run pytest tests/unit/podcasts/min_minutesstill loads in UIHigh-level PR Summary
This PR migrates podcast brief duration specifications from minute-based to second-based storage, enabling finer granularity with new defaults of 20–30 seconds instead of 10–20 minutes. It adds a duration unit picker (seconds/minutes/hours) to the brief review UI with proper clamping between 15 seconds and 24 hours, while maintaining backward compatibility with existing podcast briefs stored using the legacy
min_minutesandmax_minutesfields through automatic migration logic on both backend and frontend.⏱️ Estimated Review Time: 15-30 minutes
💡 Review Order Suggestion
surfsense_backend/app/podcasts/duration_limits.pysurfsense_backend/app/podcasts/schemas/spec.pysurfsense_web/contracts/types/podcast.types.tssurfsense_backend/app/podcasts/api/schemas.pysurfsense_backend/app/podcasts/generation/brief/config.pysurfsense_backend/app/podcasts/generation/brief/propose.pysurfsense_backend/app/podcasts/generation/brief/nodes.pysurfsense_backend/app/podcasts/generation/transcript/nodes.pysurfsense_backend/app/podcasts/api/routes.pysurfsense_web/components/tool-ui/podcast/brief-review.tsxsurfsense_backend/tests/unit/podcasts/test_spec.pysurfsense_backend/tests/unit/podcasts/conftest.pysurfsense_backend/tests/integration/podcasts/conftest.pysurfsense_backend/tests/unit/podcasts/test_renderer.pySummary by CodeRabbit